Skip to content

Conversation

@0x404
Copy link
Contributor

@0x404 0x404 commented Apr 25, 2025

Description

Some hf dataset, such as openai/gsm8k has multi config (e.g., "main" and "socratic"), directly load dataset will lead to error. Instead, we should use the user specified config_name if provided, and if not, we simply try to load_dataset with dataset name.

This PR also update examples/grpo_gsm8k/gsm8k.yaml's buffer.train_dataset.path from sqlite:////gsm8k.db to sqlite:///gsm8k.db, since sqlite:////gsm8k.db will create the db file under the root directory (some may have no permission under a server's root directory), instead sqlite:///gsm8k.db will create the db file under the current direcotry.

Checklist

Please check the following items before code is ready to be reviewed.

  • Code has passed all tests
  • Docstrings have been added/updated in Google Style
  • Documentation has been updated
  • Code is ready for review

@pan-x-c
Copy link
Collaborator

pan-x-c commented Apr 25, 2025

/run-unittest

@github-actions
Copy link

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
13 13 0 0 0 0 0 97ms

Failed Tests

No failed tests ✨

Flaky Tests

No flaky tests ✨

Skipped

No skipped tests ✨

Tests

Test Name Status Flaky Duration
tests/buffer/sql_test.py::TestSQLBuffer::test_create_sql_buffer 1ms
tests/common/config_test.py::TestConfig::test_all_examples_are_valid 1ms
tests/common/config_test.py::TestConfig::test_load_default_config 1ms
tests/common/experience_test.py::TestExperienceConversion::test_batch_conversion 1ms
tests/common/experience_test.py::TestExperienceConversion::test_experience_model_experience_conversion 1ms
tests/common/vllm_test.py::TestModelWrapperSync::test_generate 40ms
tests/common/vllm_test.py::TestModelWrapperAsync::test_generate 38ms
tests/common/vllm_test.py::TestTokenizer::test_assistant_token_mask 1ms
tests/explorer/runner_pool_test.py::RunnerPoolTest::test_runner_pool 13ms
tests/explorer/workflow_test.py::WorkflowTest::test_gsm8k_workflow 1ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_complex_workflow 1ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_fraction_workflow 1ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_workflow 1ms

Github Test Reporter by CTRF 💚

0x404 and others added 2 commits April 27, 2025 10:43
Co-authored-by: Yuchang Sun <[email protected]>
Co-authored-by: Yuchang Sun <[email protected]>
Copy link
Collaborator

@yanxi-chen yanxi-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@yanxi-chen yanxi-chen merged commit a9c650b into modelscope:main Apr 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants